Bug: TOCTOU race — uses _sandbox.kill() instead of sb.kill()

Line 524 calls _sandbox.kill() on the global reference, but the locally captured sb (line 348) is the correct sandbox reference. Between the guard check (lines 347-349, which releases the lock) and this line's lock re-acquisition, a concurrent sandbox_destroy call could have set _sandbox = None (line 526, 529), causing AttributeError. If _sandbox were reassigned by a concurrent sandbox_create, the wrong sandbox would be killed.

Replace _sandbox.kill() with sb.kill().

Bug: No-op signal handlers prevent clean shutdown

The installed handler lambda: None silently consumes SIGTERM and SIGINT instead of initiating shutdown. This means:

1. SIGINT no longer raises KeyboardInterrupt, so asyncio.run() cannot perform graceful cancellation.
2. SIGTERM no longer terminates the process — the default action would do so.
3. The atexit-registered _cleanup() never fires, so the sandbox MicroVM remains running as an orphan until its timeout expires (up to 86,400 seconds).

The comment on line 551 claims this forwards signals for "clean shutdown + atexit cleanup", but the no-op achieves the opposite. Replace with a handler that actually stops the event loop, e.g. loop.add_signal_handler(sig, loop.stop), or remove the handlers entirely.

Docstring mismatch: _sanitize_error claims to remove URLs/stack traces but only truncates

The docstring says "no stack traces or internal URLs", but the implementation only truncates at 500 characters (msg[:500]). If an SDK exception message contains an internal hostname, IP address, or internal URL within the first 500 characters, that information leaks to the MCP client.

Add explicit scrubbing of https?://... URL patterns and File "/path/" stack-trace patterns before the truncation step.

Docstring mismatch: _sanitize_path claims to reject absolute paths but doesn't

The docstring says "Reject path traversal and absolute paths outside sandbox workspace", but the implementation only checks ".." in p.parts. Absolute paths like /etc/passwd or /proc/1/cmdline pass through. Tool descriptions for sandbox_read_file and sandbox_write_file say "Relative path inside the sandbox workspace".

Either add if p.is_absolute(): raise ValueError(...) or correct the docstring to match.

Misleading success message: unverified claim about snapshot creation

The message says "A snapshot was created" but the handler only calls sb.pause() without verifying a snapshot was actually produced. If sb.pause() is a pure suspend operation that doesn't create a persistent snapshot, this is misleading.

Either verify the snapshot creation or update the message to accurately describe what sb.pause() does.

Leaks API response keys when no known keys are found

If the SDK response has none of the expected keys (sandboxID, templateID, state, cpuCount, memoryMB, startedAt), the full sorted key set is returned to the user (line 475). This leaks internal API structure and undocumented fields.

Return a generic message like "(info returned, no recognised keys)" instead.

Bug: No-op signal handlers prevent clean shutdown

The installed handler lambda: None silently consumes SIGTERM and SIGINT instead of initiating shutdown. This means:

1. SIGINT no longer raises KeyboardInterrupt, so asyncio.run() cannot perform graceful cancellation.
2. SIGTERM no longer terminates the process (default action is SIG_DFL/terminate).
3. The atexit-registered _cleanup() never fires, so the sandbox MicroVM remains running as an orphan until its timeout expires (up to 86,400 seconds).

The comment on line 551 claims this forwards signals for "clean shutdown + atexit cleanup", but the no-op achieves the opposite. Replace with a handler that actually stops the event loop, e.g. loop.add_signal_handler(sig, loop.stop), or remove the handlers entirely to rely on default behavior.

Docstring mismatch: _sanitize_error claims to remove stack traces and internal URLs but only truncates

The docstring says "no stack traces or internal URLs", but the implementation only truncates at 500 characters (msg[:500]). If an exception message (particularly from the SDK's _check_response which may include resp.text from CubeAPI) contains an internal hostname, IP, or internal URL within the first 500 characters, that information is leaked to Claude Code.

Consider adding explicit scrubbing of https?://... URL patterns and File "/path/" stack-trace patterns before truncation.

Docstring mismatch: _sanitize_path claims to reject absolute paths but doesn't

The docstring says "Reject path traversal and absolute paths outside sandbox workspace", but the implementation only checks ".." in p.parts. Absolute paths like /etc/passwd or /proc/1/cmdline pass through silently. The tool descriptions for sandbox_read_file and sandbox_write_file say "Relative path inside the sandbox workspace", reinforcing the expectation.

Either add if p.is_absolute(): raise ValueError(...) or correct the docstring to match the actual behavior.

Misleading success message: unverified claim about snapshot creation

The success message says "A snapshot was created and will be accessible via snapshot management" but the handler only calls sb.pause() and does not verify a snapshot was actually created. If sb.pause() is a pure suspend operation that does not produce a persistent snapshot, this message is misleading.

Verify the snapshot creation (e.g., check the return value or call sb.create_snapshot explicitly before pausing), or update the message to accurately describe what sb.pause() does.

Leaks API response keys when no known keys are found

If the SDK returns a response with none of the expected keys (sandboxID, templateID, state, cpuCount, memoryMB, startedAt), the full sorted key set is returned to the user (line 475). This could leak internal API structure or undocumented fields.

Return a generic message like "(info returned, no recognised keys)" instead of exposing the raw key set.

Race condition: _cleanup() reads _sandbox without acquiring _lock

_cleanup() reads _sandbox and calls sb.kill() without holding _lock. The handler registered via atexit may run during shutdown while async tasks are still executing and mutating _sandbox under the lock. This is a data race — a torn read and/or concurrent sb.kill() calls.

Recommendation: Acquire _lock before accessing _sandbox in _cleanup().

Missing timeout on Sandbox.create — can permanently consume thread-pool slot

asyncio.to_thread(Sandbox.create, ...) submits the call to a thread-pool. If the CubeAPI endpoint is unreachable or slow, the underlying requests.post() call blocks the thread-pool thread indefinitely (Python stdlib HTTP has no default timeout). Repeated blocked calls would exhaust the thread pool (~5-21 threads), making the server unresponsive to ALL tool calls.

Additionally, the _lock is held during the entire multi-second creation (including the HTTP POST and sandbox boot), blocking other tool requests that queue behind it.

Recommendation: Wrap with asyncio.wait_for(..., timeout=60) and consider releasing the lock during the creation call (re-check _sandbox is None after the call completes).

TOCTOU: _sandbox reference captured under lock but used outside it for all tool handlers

sb = _sandbox is read under the lock, then every tool handler (run_code, run_command, read_file, write_file, destroy, etc.) uses sb outside the lock. A concurrent sandbox_destroy call can set _sandbox = None (under the lock) while another handler is mid-way through sb.run_code() or sb.files.read() on a now-destroyed sandbox. The local sb prevents a None dereference, but the underlying API call may fail with a confusing 404 or connection error.

Recommendation: Either re-read _sandbox under the lock before each SDK operation, or use a reader-writer pattern where destroy serializes behind create/run operations.

Blocking SDK call inside async function will hang the event loop

This "Minimal MCP Server" snippet shows Sandbox.create(...) called directly inside async def call_tool() without await asyncio.to_thread(...). Since Sandbox.create() is a synchronous blocking call (wraps requests.post), calling it directly in an async function blocks the asyncio event loop, preventing the server from handling any other requests.

The actual mcp_server.py correctly wraps this with await asyncio.to_thread(Sandbox.create, ...). As this is labeled "Minimal" and readers may copy it verbatim, it should use the same non-blocking pattern.

Broken relative link to Python SDK README

../sdk/python/README.md from inside examples/claude-code-sandbox/ resolves to examples/sdk/python/README.md, which does not exist. The correct relative path should be ../../sdk/python/README.md (go up two levels to repo root, then into sdk/python/).

The same issue exists in README_zh.md at line 254.

Broken relative link to claude-code-sandbox

../../../examples/claude-code-sandbox/ from examples/pi-agent-sandbox/ resolves one level above the repo root. The correct relative path is ../claude-code-sandbox/ (examples/pi-agent-sandbox/ → .. → examples/ → claude-code-sandbox/).

Race condition: _cleanup() reads _sandbox without acquiring _lock

_cleanup() at line 65 reads the module-level _sandbox variable and then calls sb.kill() without holding _lock. Since it's registered via atexit, it may run during interpreter shutdown while async tasks are concurrently modifying _sandbox under _lock in call_tool(). This can produce a torn read or a concurrent sb.kill() racing with a destroy call.

Recommendation: Acquire _lock before reading _sandbox in _cleanup(), or mark it as a known limitation with a comment.